[BREAKING] MAINT: enforce _async suffix on async functions across pyrit/#1889
Merged
romanlutz merged 25 commits intoJun 2, 2026
Merged
Conversation
The style guide mandates that every `async def` in `pyrit/` end with the `_async` suffix. There was previously no automated enforcement, so the rule relied entirely on reviewer attention and regressed regularly. This change adds a pre-commit hook (`build_scripts/check_async_suffix.py`) that walks every `pyrit/**/*.py` file with `ast` and flags every `AsyncFunctionDef` whose name doesn't end in `_async` and isn't exempted. To avoid blocking on a one-shot mass cleanup, the hook uses a transitional allowlist (`build_scripts/async_suffix_baseline.txt`) of 168 pre-existing violations -- mirroring the `tests/unit/models/test_import_boundary.py` pattern. The baseline must shrink monotonically; the hook reports drift if a baseline entry no longer matches a violation in the source. Exemption mechanisms (in priority order): 1. Name ends with `_async`. 2. Name starts with `__a` (async dunders: `__aenter__`, `__aexit__`, `__aiter__`, `__anext__`). 3. Name is in the hard-coded `_FRAMEWORK_EXEMPT_NAMES` set (`lifespan`, `dispatch`, `__call__`). 4. The `async def` line carries a `# pyrit-async-suffix-exempt` trailing comment for one-off exceptions. 5. The `(path, name)` pair is present in the baseline (transitional only). The style guide is updated to document the marker syntax and the baseline shrinkage contract. Follow-up commits will rename the existing 168 violations subpackage-by-subpackage, each removing its corresponding baseline entries. No deprecation shims are added by this commit. `removed_in` is not applicable here. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Renames every async def in pyrit/auth/ to end in _async per the style guide; drains the 12 corresponding entries from the enforcement baseline. Public-API methods (with backward-compatible shim, removed_in=0.16.0): - AzureStorageAuth.get_user_delegation_key -> _async - AzureStorageAuth.get_sas_token -> _async - CopilotAuthenticator.get_claims -> _async - ManualCopilotAuthenticator.get_claims -> _async Private methods (renamed in place, no shim): - CopilotAuthenticator._get_cached_token_if_available_and_valid -> _async - CopilotAuthenticator._fetch_access_token_with_playwright -> _async - CopilotAuthenticator._run_playwright_in_thread -> _async - CopilotAuthenticator._run_playwright_browser_automation -> _async Closures renamed (no shim needed): - azure_auth.async_token_provider -> _async - copilot_authenticator.response_handler -> _async External-protocol methods marked # pyrit-async-suffix-exempt (Azure SDK AsyncTokenCredential contract): - AsyncTokenProviderCredential.get_token - AsyncTokenProviderCredential.close Internal callers updated: - pyrit/models/storage_io.py - pyrit/prompt_target/azure_blob_storage_target.py - pyrit/prompt_target/websocket_copilot_target.py - tests/unit/auth/, tests/unit/models/test_storage_io.py, tests/unit/prompt_target/target/test_websocket_copilot_target.py Enforcement script tweak: - check_async_suffix.py: scan the entire async-def header (not just the first line) for the # pyrit-async-suffix-exempt marker, so the marker survives when ruff splits a long signature across lines. - Docstring updated to call out deprecation shims as a legitimate reason to use the marker. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drains the 39 backend entries from the enforcement baseline.
Private service methods (renamed in place, no shim):
- AttackService._store_prepended_messages -> _async
- ConverterService._apply_converters -> _async
Both are internal helpers with a single in-file caller; no external
references exist in tests or other packages.
FastAPI dispatch callbacks marked # pyrit-async-suffix-exempt
(framework-determined names that surface in OpenAPI as operation IDs
and through `@app.exception_handler` registration; the framework
dispatches by URL or exception class, not function name):
- 6 handlers in middleware/error_handlers.py
- 31 route handlers across routes/{attacks,converters,initializers,
labels,scenarios,targets}.py
No behavior changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drains the 1 cli entry from the enforcement baseline. - PyRITApiClient._get_json -> _get_json_async (private; renamed in place, no shim, internal helper called from 7 sites in the same file) - Updated the 7 in-file call sites and a stale comment in the test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… of sweep) Drains the 6 common entries from the enforcement baseline. These six functions were already deprecation shims (delegating to a `*_async` partner with `print_deprecation_message`) but had no exempt marker, so the enforcement script flagged them. Adding the `# pyrit-async-suffix-exempt` marker reflects the intent: keep the backward-compatible name available for one release cycle while still enforcing the suffix rule for new code. - pyrit.common.data_url_converter.convert_local_image_to_data_url - pyrit.common.display_response.display_image_response - pyrit.common.download_hf_model.download_specific_files - pyrit.common.download_hf_model.download_chunk - pyrit.common.download_hf_model.download_file - pyrit.common.download_hf_model.download_files Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drains the 9 datasets entries from the enforcement baseline.
OVERRIDE-SETs (renamed atomically on the ABC + every subclass + every
caller; private, no shim):
- SeedDatasetProvider._parse_metadata -> _async
(overridden in local_dataset_loader and remote_dataset_loader)
- _RemoteDatasetLoader._fetch_from_huggingface -> _async
(called from 25 remote-dataset subclasses)
- _RemoteDatasetLoader._fetch_zip_from_url -> _async
(called from moral_integrity_corpus_dataset)
- _EquityMedQADataset._get_sub_dataset -> _async
Nested closures renamed (no shim needed):
- SeedDatasetProvider.fetch_datasets_async.fetch_single_dataset
-> _async
- SeedDatasetProvider.fetch_datasets_async.fetch_with_semaphore
-> _async
Existing public-API shim marked exempt:
- SeedDatasetProvider.fetch_dataset (already delegates to the _async
partner with print_deprecation_message; marker reflects that intent)
Test mocks updated: ~25 tests in tests/unit/datasets/ use
`patch.object(loader, "_fetch_from_huggingface", ...)` — all
string-literal references updated to `_fetch_from_huggingface_async`.
Doc updated:
- doc/code/datasets/4_dataset_coding.{py,ipynb} `_fetch_from_huggingface`
-> `_async` in the example dataset implementation.
No behavior changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drain 24 entries from the async_suffix_baseline by renaming async methods/closures in pyrit/executor to match the style-guide convention. Renames: - StrategyEventHandler.on_event -> on_event_async (ABC + 3 subclass overrides in attack_strategy, prompt_generator_strategy, workflow_strategy; 1 dispatch site; ~20 test references). - AttackStrategy._on, _on_pre_execute, _on_post_execute -> *_async (template-method hooks; _events dict in attack_strategy.py rewritten). - WorkflowStrategy._on_pre_validate, _on_post_validate, _on_pre_setup, _on_post_setup, _on_pre_execute, _on_post_execute, _on_pre_teardown, _on_post_teardown, _on_error -> *_async (template-method hooks; _events dict in workflow_strategy.py rewritten). - Strategy._handle_event -> _handle_event_async (private; in-file callers updated). - Strategy._execution_context -> _execution_context_async (private asynccontextmanager; async with self._execution_context(context) caller updated). - RedTeamingAttack._build_adversarial_prompt -> _async (only the async variant; the synchronous Crescendo._build_adversarial_prompt is left untouched). - RolePlayAttack._get_conversation_start -> _async. - FairnessBiasBenchmark._run_experiment -> _async. - AttackExecutor closures build_params, run_one -> *_async. - AttackParameters.from_seed_group_async_wrapper closure -> from_seed_group_wrapper_async. All renames are private/closure or limited to the executor subpackage, so no deprecation shims are added. Tests and the dispatch site are updated in lockstep. tests/unit/executor passes (879 passed). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…r to use _async suffix Drain 3 entries from the async_suffix_baseline: - MemoryInterface._serialize_seed_value -> _serialize_seed_value_async (private; 1 in-file caller updated; mock target in tests/unit/memory/memory_interface/test_interface_seed_prompts.py updated). - ChatMessageNormalizer._convert_audio_to_input_audio -> _convert_audio_to_input_audio_async (private; 1 in-file caller updated). - apply_system_message_behavior -> apply_system_message_behavior_async (module-level public helper; deprecation shim added that delegates to the new name and emits print_deprecation_message with removed_in='0.16.0'). Internal callers in chat_message_normalizer and tokenizer_template_normalizer updated to use the new name; the unit test is updated to import the new name so it does not trigger the deprecation warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…odels) Drains 22 entries from build_scripts/async_suffix_baseline.txt by renaming all `async def` methods on the StorageIO and DataTypeSerializer base classes (and their subclasses) to end in `_async`, per the project's `_async`-suffix style rule. StorageIO ABC (and DiskStorageIO, AzureBlobStorageIO subclasses): - read_file -> read_file_async - write_file -> write_file_async - path_exists -> path_exists_async - is_file -> is_file_async - create_directory_if_not_exists -> create_directory_if_not_exists_async DataTypeSerializer ABC (and all subclasses): - save_data -> save_data_async - save_b64_image -> save_b64_image_async - save_formatted_audio -> save_formatted_audio_async - read_data -> read_data_async - read_data_base64 -> read_data_base64_async - get_sha256 -> get_sha256_async - get_data_filename -> get_data_filename_async For every public API method, a deprecation shim is added that calls `print_deprecation_message(..., removed_in="0.16.0")` and delegates to the new `_async` name. Each shim is marked with `# pyrit-async-suffix-exempt` so the enforcement hook does not flag the alias itself. All internal callers in `pyrit/` and `tests/unit/` are updated to use the new `_async` names. No behavioral changes; this is a pure-rename refactor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…mpt (PR 10) `ScorerPrinterBase.print_objective_scorer` and `print_harm_scorer` are intentional deprecation shims that delegate to `write_async` and emit `print_deprecation_message(..., removed_in="0.16.0")`. They are not renaming candidates (the legacy name is the whole point of the shim), so mark them with `# pyrit-async-suffix-exempt` and drop them from the baseline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nc suffix (PR 11)
Drains 7 entries from build_scripts/async_suffix_baseline.txt by renaming
private (`_*`) async methods in pyrit/prompt_converter/ to end in `_async`.
All renamed methods are private and never overridden outside pyrit/, so no
deprecation shims are added (per the agreed sweep convention for privates).
- PromptConverter._replace_text_match -> _replace_text_match_async
- BaseImageToImageConverter._read_image_from_url -> _read_image_from_url_async
- ImageCompressionConverter._read_image_from_url -> _read_image_from_url_async
(template-method override; both ABC and subclass renamed atomically)
- ImageCompressionConverter._handle_original_image_fallback -> _handle_original_image_fallback_async
- AddImageToVideoConverter._add_image_to_video -> _add_image_to_video_async
- PDFConverter._serialize_pdf -> _serialize_pdf_async
- TransparencyAttackConverter._save_blended_image -> _save_blended_image_async
All internal callers and test `patch.object`/direct-call sites updated.
No behavioral changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ix (PR 12) Drains 3 entries from build_scripts/async_suffix_baseline.txt. PromptNormalizer: - convert_values -> convert_values_async (PUBLIC, shim) - add_prepended_conversation_to_memory -> add_prepended_conversation_to_memory_async (PUBLIC, shim) - _calc_hash -> _calc_hash_async (PRIVATE, no shim) The two public methods get deprecation shims marked `# pyrit-async-suffix-exempt` that call `print_deprecation_message(..., removed_in="0.16.0")` and delegate to the renamed `_async` versions. Internal callers and tests (in both prompt_normalizer/ and executor/attack/component/conversation_manager.py) updated to use the new names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Renames 14 async methods across pyrit/prompt_target/ to comply with the style-guide _async suffix rule. Adds deprecation shims for public methods. Renamed (private, no shim): - pyrit/prompt_target/playwright_copilot_target.py: 7 methods - pyrit/prompt_target/websocket_copilot_target.py: 2 methods - pyrit/prompt_target/common/utils.py: set_max_rpm (nested closure) Renamed (public, with deprecation shim removed_in=0.16.0): - pyrit/prompt_target/gandalf_target.py: check_password - pyrit/prompt_target/text_target.py: cleanup_target - pyrit/prompt_target/hugging_face/hugging_face_chat_target.py: load_model_and_tokenizer - pyrit/prompt_target/openai/openai_realtime_target.py: cleanup_target Updated callers in tests/unit/prompt_target/ and docs in doc/code/targets/. Baseline drained 14 entries (40 -> 26). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…x (PR 14) Renames 21 async methods across pyrit/prompt_target/openai/ to comply with the style-guide _async suffix rule. Adds deprecation shims for public realtime-target methods. Renamed (private, no shim — OVERRIDE-SETs renamed atomically): - _construct_message_from_response: openai_target ABC + 7 subclass overrides (chat, completion, image, realtime, response, tts, video targets) - _construct_request_body: openai_chat_target + openai_response_target - _construct_input_item_from_piece, _execute_call_section (response_target) - _handle_openai_request (openai_target) - _save_video_response (openai_video_target) - _get_image_bytes (openai_image_target) Renamed (public RealtimeTarget API, with deprecation shims removed_in=0.16.0): - connect, send_config, save_audio, cleanup_conversation, send_response_create, receive_events Updated callers in tests/unit/prompt_target/target/. Baseline drained 21 entries (26 -> 5). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Renames the nested `worker` closure to `worker_async` in pyrit/scenario/core/scenario.py to comply with the style-guide _async suffix rule. Updates the single in-file caller. Baseline drained 1 entry (5 -> 4). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Renames the remaining 4 private async methods across pyrit/score/ to comply with the style-guide _async suffix rule. No shims needed (all private). Renamed (private, OVERRIDE-SETs renamed atomically): - _score_value_with_llm: scorer.py ABC + float_scale_scorer.py override (callers in 6 production scorers + several tests via patch.object) - _check_for_password_in_conversation: gandalf_scorer.py - _get_base64_image_data: azure_content_filter_scorer.py Also updated string-literal references to the old name in: - pyrit/exceptions/exceptions_helpers.py docstring example - tests/unit/exceptions/test_exceptions_helpers.py retry_state.fn.__name__ fixture (production code now reports the renamed function name) Baseline drained 4 entries (4 -> 0). All baseline-deferred violations have now been cleaned up. A follow-up commit will delete the baseline file and the baseline-loading code in check_async_suffix.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
All pre-existing async-suffix violations have been cleaned up across the preceding 16 PRs. The transitional baseline allowlist and its loading code in check_async_suffix.py are no longer needed. Changes: - Delete build_scripts/async_suffix_baseline.txt - Remove --write-baseline flag, _load_baseline(), _write_baseline(), _report_failures() drift reporting, and argparse from build_scripts/check_async_suffix.py The hook now simply scans pyrit/ and fails on any AsyncFunctionDef whose name doesn't end in _async (unless framework-mandated via the small hard-coded exempt set, an async dunder like __aenter__, or a per-line `# pyrit-async-suffix-exempt` marker for deprecation shims). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Per review feedback: this nested closure's name already leads with "async", so the redundant `_async` suffix adds noise without clarifying that it is async. Restore the original name and add a `# pyrit-async-suffix-exempt` marker so future audits see the exemption is intentional. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
After merging main, several new files/tests called methods using their pre-rename names. Update them to use the *_async suffixed names: - _parse_metadata -> _parse_metadata_async (test_seed_dataset_provider.py, docs) - _fetch_from_huggingface -> _fetch_from_huggingface_async (jailbreakv_28k_dataset.py, jailbreakv_redteam_2k_dataset.py and their tests, test_coconot_dataset.py) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Diff coverage on PR microsoft#1889 was failing at 83% due to untested deprecation shim bodies for methods renamed with the _async suffix. Add minimal pytest.warns(DeprecationWarning) tests for each shim that delegates to the new *_async name, plus focused tests for the two non-shim renamed code paths in azure_content_filter_scorer and self_ask_question_answer_scorer that had pre-existing coverage gaps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rlundeen2
reviewed
Jun 2, 2026
rlundeen2
reviewed
Jun 2, 2026
rlundeen2
reviewed
Jun 2, 2026
- Remove stale baseline-file paragraph from style guide (the file was removed in the final commit of the sweep). - Trim the pre-commit `files` regex to `^pyrit/.*\\.py$` now that the baseline file no longer exists. - Treat SyntaxError as a violation in check_async_suffix.py instead of silently returning [], so an unparseable file can't escape the check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…anlutz-async-suffix-sweep # Conflicts: # pyrit/models/message_piece.py # pyrit/prompt_converter/add_image_to_video_converter.py # tests/unit/prompt_target/target/test_realtime_target.py
…anlutz-async-suffix-sweep # Conflicts: # pyrit/models/data_type_serializer.py # tests/unit/models/test_data_type_serializer.py # tests/unit/prompt_converter/test_add_image_video_converter.py
rlundeen2
approved these changes
Jun 2, 2026
romanlutz
added a commit
to romanlutz/PyRIT
that referenced
this pull request
Jun 3, 2026
…nch-dataset-loader Resolved conflicts: - doc/bibliography.md (kept @liu2024mmsafetybench alongside main's @li2024mossbench and @luo2024jailbreakv) - pyrit/datasets/seed_datasets/remote/__init__.py (kept MMSafetyBench entries alongside MossBench entries) Updated _MMSafetyBenchDataset and its tests to call the renamed _fetch_from_huggingface_async (renamed by PR microsoft#1889's _async-suffix enforcement). Also replaced two :class:\SeedDataset\ Sphinx roles with plain double-backticks to satisfy the check-no-rest-roles pre-commit hook. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
romanlutz
added a commit
to romanlutz/PyRIT
that referenced
this pull request
Jun 3, 2026
Resolved conflicts: - pyrit/datasets/seed_datasets/remote/__init__.py — kept both MaskQuestionArchetype (HEAD) and MossBenchOversensitivityType (main) in __all__. - doc/bibliography.md — merged the new MASK citation key into the upstream's larger citation list. Followups for upstream changes: - mask_dataset.py + test_mask_dataset.py: renamed _fetch_from_huggingface -> _fetch_from_huggingface_async to match the async-suffix sweep (microsoft#1889). - seed_metadata.py: added "honesty" to RECOMMENDED_TAGS so MASK's cross-cutting tag passes the metadata-coverage check added by microsoft#1780. Verification: 836 unit dataset tests pass; pre-commit clean on all touched files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The style guide (
.github/instructions/style-guide.instructions.md§1) mandates that everyasync definpyrit/end in_async, but an audit turned up ~30 violations acrossauth/,backend/,cli/,datasets/,executor/,prompt_target/,prompt_converter/, and others. The rule had no automated enforcement, so regressions slipped in through normal review. This PR fixes the violations and lands a pre-commit hook so it cannot regress again.Approach
Rename every offending async function to add the
_asyncsuffix, in both definitions and call sites (production + tests + notebooks).Add deprecation shims for renamed public APIs using the existing
pyrit.common.deprecation.print_deprecation_messagepattern, withremoved_in="0.16.0"(current0.14.0.dev0+ 2 minor):Private methods (
_foo) are renamed without shims since they are not a public surface.Add
build_scripts/check_async_suffix.py-- an AST-based pre-commit hook that flags anyasync definpyrit/whose name does not end in_async. Three escape hatches keep it pragmatic:__call__, async__a*dunders, FastAPIlifespan, Starlettedispatch).# pyrit-async-suffix-exemptmarker for site-specific exemptions (FastAPI routes, Azure SDK protocol implementations, deprecation shims themselves, etc.), so every exception is visible at the violation site rather than hidden in a central allowlist.Update the style guide to point at the new hook so the rule and its enforcement are co-located.
What is marked exempt and why
__aenter__,__aexit__,__aiter__,__anext__,__call__lifespan, Starlette middlewaredispatchpyrit/backend/routes/*.py-- name is the OpenAPI/URL contractpyrit/backend/middleware/error_handlers.pyazure.core.credentials_async.AsyncTokenCredential.get_token/close(auth), Playwrightpage.on("response", ...)handlerasync def; the shim is the exemptionThe shim-bearing public methods are listed in the per-commit messages (per subpackage, PRs 2 through 16 in the commit log).
Breaking change rationale
DeprecationWarninguntil0.16.0.Scorer._score_value_with_llm,WorkflowStrategy._on_*event hooks, etc. will need to rename their overrides. Tagging[BREAKING]so this shows up in release notes.Anything reviewers should look at carefully
pyrit/auth/azure_auth.py:152-- the nested closureasync_token_provideris exempted (already leads withasync); this was a review-feedback decision.pyrit/executor/workflow/core/workflow_strategy.py-- the renamed_on_*template-method hooks are part of a class hierarchy; all known subclass overrides were updated, but downstream consumers should double-check.pyrit/prompt_target/openai/openai_realtime_target.py-- 6 public methods (connect,send_config,save_audio,cleanup_conversation,send_response_create,receive_events) got shims; the realtime target API surface is wider after this change.pyrit/exceptions/exceptions_helpers.py:54-- the only non-rename production diff is a comment string update (_score_value_with_llm->_score_value_with_llm_async) so the# e.g.example matches the new function name.Tests and Documentation
doc/code/datasets/4_dataset_coding.{py,ipynb}anddoc/code/targets/realtime_target.{py,ipynb}were updated to call the renamed_asyncmethods directly. They were re-rendered withjupytext --update --to ipynbso cell outputs are preserved..pre-commit-config.yamlso every PR going forward will fail fast if a newasync defviolates the rule.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com